test: migrate e2e/mint to system tests #22294
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request involves the deletion of end-to-end (E2E) test files for the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
tests/systemtests/mint_test.go (1)
40-43: Offer assistance to resolve thequeryAtHeightdiscrepancyThe TODO comment suggests uncertainty about why
queryAtHeightdiffers between v1 and v2. I can help investigate this issue to understand the underlying cause and update the test accordingly, ensuring consistent behavior across versions.Would you like me to look into the difference in
queryAtHeightvalues between v1 and v2 and suggest code adjustments?
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (4)
- tests/e2e/mint/cli_test.go (0 hunks)
- tests/e2e/mint/grpc.go (0 hunks)
- tests/e2e/mint/suite.go (0 hunks)
- tests/systemtests/mint_test.go (1 hunks)
💤 Files with no reviewable changes (3)
- tests/e2e/mint/cli_test.go
- tests/e2e/mint/grpc.go
- tests/e2e/mint/suite.go
🧰 Additional context used
📓 Path-based instructions (1)
tests/systemtests/mint_test.go (3)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
🔇 Additional comments (1)
tests/systemtests/mint_test.go (1)
12-97: Implementation ofTestMintQueriesis comprehensiveThe test function covers the necessary scenarios for minting queries, including both gRPC and CLI interfaces. The test cases are well-defined, and assertions use
require.JSONEqappropriately.
tests/systemtests/mint_test.go
Outdated
| blockHeightHeader := "x-cosmos-block-height" | ||
| queryAtHeight := "1" | ||
|
|
||
| // TODO: check why difference in values basequeryAtH on height between v1 and v2 |
There was a problem hiding this comment.
Typo in comment needs correction
There's a typo in the comment at line 40. The phrase "check why difference in values basequeryAtH on height between v1 and v2" should be corrected for clarity. It should likely read: "check why there is a difference in values of queryAtHeight based on height between v1 and v2."
| // TODO: remove below check once grpc gateway is implemented in v2 | ||
| if isV2() { | ||
| return | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use t.Skip() to properly skip tests
Instead of returning early when isV2() is true, use t.Skip() to indicate that the test is being intentionally skipped. This approach provides clearer test output and accurately reflects the test status.
Apply this change:
- return
+ t.Skip("Skipping test until gRPC gateway is implemented in v2")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // TODO: remove below check once grpc gateway is implemented in v2 | |
| if isV2() { | |
| return | |
| } | |
| // TODO: remove below check once grpc gateway is implemented in v2 | |
| if isV2() { | |
| t.Skip("Skipping test until gRPC gateway is implemented in v2") | |
| } |
akhilkumarpilli
added
the
backport/v0.52.x
|
|
||
| // TODO: check why difference in values basequeryAtH on height between v1 and v2 | ||
| if isV2() { | ||
| queryAtHeight = "2" |
There was a problem hiding this comment.
lets open an issue about this so we dont forget it
There was a problem hiding this comment.
Opened #22302
(cherry picked from commit 29ea9c8)
Co-authored-by: Akhil Kumar P <[email protected]>
Description
Closes: #22291
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
Bug Fixes